feat: Drill ODBC/JDBC Impersonation feature #17353

Z0ltrix · 2021-11-05T12:15:38Z

SUMMARY

Added Drill ODBC/JDBC Impersonation feature and necessary translations/docs

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Connect Superset to a Drill Cluster via ODBC/JDBC and activate Impersonation via UI.

ADDITIONAL INFORMATION

Has associated issue: Apache Drill ODBC Impersonation #17352
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2021-11-05T13:12:58Z

Codecov Report

Merging #17353 (a9dbb75) into master (561d1ac) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

❗ Current head a9dbb75 differs from pull request most recent head da4f9a1. Consider uploading reports for the commit da4f9a1 to get more accurate results

@@            Coverage Diff             @@
##           master   #17353      +/-   ##
==========================================
- Coverage   77.07%   76.91%   -0.16%     
==========================================
  Files        1036     1036              
  Lines       55756    55764       +8     
  Branches     7630     7630              
==========================================
- Hits        42972    42891      -81     
- Misses      12529    12618      +89     
  Partials      255      255

Flag	Coverage Δ
hive	`?`
mysql	`81.91% <100.00%> (+<0.01%)`	⬆️
postgres	`81.92% <100.00%> (+<0.01%)`	⬆️
python	`81.96% <100.00%> (-0.31%)`	⬇️
sqlite	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../CRUD/data/database/DatabaseModal/ExtraOptions.tsx	`93.18% <ø> (ø)`
superset/db_engine_specs/drill.py	`86.84% <100.00%> (+3.50%)`	⬆️
superset/db_engines/hive.py	`0.00% <0.00%> (-85.19%)`	⬇️
superset/db_engine_specs/hive.py	`69.76% <0.00%> (-17.06%)`	⬇️
superset/db_engine_specs/sqlite.py	`91.89% <0.00%> (-5.41%)`	⬇️
superset/connectors/sqla/utils.py	`88.23% <0.00%> (-3.93%)`	⬇️
superset/utils/celery.py	`86.20% <0.00%> (-3.45%)`	⬇️
superset/views/database/mixins.py	`81.03% <0.00%> (-1.73%)`	⬇️
superset/result_set.py	`96.77% <0.00%> (-1.62%)`	⬇️
superset/db_engine_specs/base.py	`87.62% <0.00%> (-0.97%)`	⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 561d1ac...da4f9a1. Read the comment docs.

villebro · 2021-11-05T14:10:53Z

Thanks for the PR @Z0ltrix ! This LGTM, but you need to address some linting issues (lines too long etc, please check the python-lint check). Ping @cgivre if you have time to also review this as you're most familiar with Drill.

cgivre · 2021-11-05T14:18:46Z

@villebro On it!

superset/db_engine_specs/drill.py

Z0ltrix · 2021-11-07T20:17:05Z

Thanks for the PR @Z0ltrix ! This LGTM, but you need to address some linting issues (lines too long etc, please check the python-lint check). Ping @cgivre if you have time to also review this as you're most familiar with Drill.

Hi @villebro ,
code cleanup done... at least pycharm isnt complaining about this part.
Everything else?
Regards,
Christian

villebro

One comment regarding the jdbc case as pointed out by @eschutho . I'd still like to hear @cgivre 's thoughts, and a unit test to cover both odbc and jdbc cases would be super nice.

superset/db_engine_specs/drill.py

cgivre

LGTM +1
Thanks for including the JDBC impersonation as well.

villebro · 2021-11-08T18:45:45Z

There appears to be some unrelated flakiness on CI. Let me look into what's going on..

villebro · 2021-11-09T07:12:33Z

@Z0ltrix can you rebase the PR? The flaky test has now been addressed, so this should now pass CI.

Z0ltrix · 2021-11-09T09:03:45Z

@Z0ltrix can you rebase the PR? The flaky test has now been addressed, so this should now pass CI.

@villebro merged with current master and added some unittest.

villebro

LGTM, thanks for the unit tests; great template to add to other connectors that support impersonation! ❤️

villebro · 2021-11-09T10:44:35Z

@Z0ltrix there's one remaining linting error that needs to be resolved before we can hit merge

Z0ltrix · 2021-11-09T10:54:36Z

@Z0ltrix there's one remaining linting error that needs to be resolved before we can hit merge

@villebro reformatted with black formatter, hope this helps

* Added Drill ODBC Impersonation feature and necessary translations/docs * Code Cleanup * add jdbc impersonation_target parameter * add unittests for DrillEngineSpec.modify_url_for_impersonation method * reformat test_drill.py with black formatter * run pre-commit locally Co-authored-by: Christian Pfarr <[email protected]> Co-authored-by: Christian Pfarr <[email protected]>

* Added Drill ODBC Impersonation feature and necessary translations/docs * Code Cleanup * add jdbc impersonation_target parameter * add unittests for DrillEngineSpec.modify_url_for_impersonation method * reformat test_drill.py with black formatter * run pre-commit locally Co-authored-by: Christian Pfarr <[email protected]> Co-authored-by: Christian Pfarr <[email protected]> (cherry picked from commit 333b137)

* Added Drill ODBC Impersonation feature and necessary translations/docs * Code Cleanup * add jdbc impersonation_target parameter * add unittests for DrillEngineSpec.modify_url_for_impersonation method * reformat test_drill.py with black formatter * run pre-commit locally Co-authored-by: Christian Pfarr <[email protected]> Co-authored-by: Christian Pfarr <[email protected]>

Added Drill ODBC Impersonation feature and necessary translations/docs

110720e

pull-request-size bot added the size/L label Nov 5, 2021

Z0ltrix changed the title ~~Drill ODBC Impersonation feature~~ (feat): Drill ODBC Impersonation feature Nov 5, 2021

Z0ltrix changed the title ~~(feat): Drill ODBC Impersonation feature~~ feat: Drill ODBC Impersonation feature Nov 5, 2021

eschutho reviewed Nov 5, 2021

View reviewed changes

superset/db_engine_specs/drill.py Show resolved Hide resolved

Code Cleanup

0d11745

villebro reviewed Nov 8, 2021

View reviewed changes

superset/db_engine_specs/drill.py Show resolved Hide resolved

add jdbc impersonation_target parameter

c12cc44

Z0ltrix changed the title ~~feat: Drill ODBC Impersonation feature~~ feat: Drill ODBC/JDBC Impersonation feature Nov 8, 2021

cgivre approved these changes Nov 8, 2021

View reviewed changes

Z0ltrix and others added 2 commits November 9, 2021 08:28

Merge branch 'apache:master' into dill-impersonation

d64cbb7

add unittests for DrillEngineSpec.modify_url_for_impersonation method

5e49f1f

villebro approved these changes Nov 9, 2021

View reviewed changes

reformat test_drill.py with black formatter

668e1cc

run pre-commit locally

da4f9a1

villebro merged commit 333b137 into apache:master Nov 10, 2021

villebro added the v1.4 label Nov 10, 2021

Z0ltrix mentioned this pull request Nov 10, 2021

Apache Drill ODBC Impersonation #17352

Closed

mistercrunch added 🍒 1.4.0 🍒 1.4.1 🍒 1.4.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Drill ODBC/JDBC Impersonation feature #17353

feat: Drill ODBC/JDBC Impersonation feature #17353

Z0ltrix commented Nov 5, 2021 •

edited

Loading

codecov bot commented Nov 5, 2021 •

edited

Loading

villebro commented Nov 5, 2021

cgivre commented Nov 5, 2021

Z0ltrix commented Nov 7, 2021

villebro left a comment

cgivre left a comment

villebro commented Nov 8, 2021

villebro commented Nov 9, 2021

Z0ltrix commented Nov 9, 2021

villebro left a comment

villebro commented Nov 9, 2021

Z0ltrix commented Nov 9, 2021

feat: Drill ODBC/JDBC Impersonation feature #17353

feat: Drill ODBC/JDBC Impersonation feature #17353

Conversation

Z0ltrix commented Nov 5, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Nov 5, 2021 • edited Loading

Codecov Report

villebro commented Nov 5, 2021

cgivre commented Nov 5, 2021

Z0ltrix commented Nov 7, 2021

villebro left a comment

Choose a reason for hiding this comment

cgivre left a comment

Choose a reason for hiding this comment

villebro commented Nov 8, 2021

villebro commented Nov 9, 2021

Z0ltrix commented Nov 9, 2021

villebro left a comment

Choose a reason for hiding this comment

villebro commented Nov 9, 2021

Z0ltrix commented Nov 9, 2021

Z0ltrix commented Nov 5, 2021 •

edited

Loading

codecov bot commented Nov 5, 2021 •

edited

Loading